-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initialize variables to avoid the warning message when compiling #753
Initialize variables to avoid the warning message when compiling #753
Conversation
We can use the regional enkf ctest in PR #750 to test the changes in this PR, right? |
@RussTreadon-NOAA Yes, we can use the regional EnKF case from PR #750. But need to turn on the case manually. |
Good. So we can test if initializing variables to zero has any impact ... at least in the context of the regional enkf ctest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @ShunLiu-NOAA for making these changes. Looks good to me. Initializing variables to zero should not alter results, but it would be good to confirm this via the regional enkf ctest in PR #750 or an offline test.
Approve pending no impact result from ctest or offline test.
@RussTreadon-NOAA Thank you for reviewing the changes. I will start an offline test. |
Those added initialization are in "dummy " subroutines ( as shown by those lines
So, in my opinion, no need to do extra test to see if the will have impacts on the results if we are sure there are no changes outside of those dummy subroutines ( while the whole piece (observer_fv3reg.f90) is for those dummy subroutine) |
@ShunLiu-NOAA , what is the status of this PR? |
@ShunLiu-NOAA , what is the status of this PR? |
Test project /scratch1/NCEPDEV/da/Shun.Liu/gsi_regression/GSI/build 100% tests passed, 0 tests failed out of 6 |
@hu5970 ctest on WCOSS and HERA passed. The PR is ready to merge into develop. |
Thank you @ShunLiu-NOAA for running ctests. My previous approval of this PR remains. These changes address a WCOSS Implementation Standard
@hu5970 , is it possible for you to run the regional enkf ctest for the sake of completeness? Initializing variable to |
@RussTreadon-NOAA Thank you. I will coordinate with @hu5970 for a regional enkf test. |
I run a regression test suite with the regional EnKF case on and there are the results:
|
I also updated the ctest instruction on how to run ctest with regional enkf case. |
Description
This PR is to initialize variables in observer_fv3reg.f90 to avoid the warning message when compiling.
The change will update regional EnKF excutable. However, the current regression test can't identify the changes in regional EnKF mode. The difference between before and after this update is that no warning message appears when compiling regional EnKF.
This PR is to address #751
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist